Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Entity Analytics][9.0] Remove all legacy risk engine code and features #201810

Merged
merged 53 commits into from
Jan 10, 2025
Merged

[Entity Analytics][9.0] Remove all legacy risk engine code and features #201810

merged 53 commits into from
Jan 10, 2025
+453 −15,921

Conversation

hop-dev
Copy link
Contributor

@hop-dev hop-dev commented Nov 26, 2024 •
edited
Loading

Summary

Closes https://github.com/elastic/security-team/issues/11253
Breaking change proposal: https://github.com/elastic/dev/issues/2822

The host and user risk scoring modules or "legacy risk engine" as we often call it internally, has been superseded since v8.10.0 by the risk engine. We submitted a breaking change proposal for v9.0.0 to remove all support to this legacy approach which was approved.

In 8.18 users will be given a warning if they are still using the legacy risk engine and directed to upgrade, this is implemented in #202775.

Changes

  • we previously used the presence of the risk index to decide whether to show the "enable risk engine" button instead of the risk score table, now that we only have only one risk scoring approach, I have changed this to use the risk engine status API, if the risk engine has ever been installed we show the table.

Deletions

  • all code related to displaying legacy risk score
  • we no longer enrich alerts with legacy risk score
  • all code related to upgrading to the "new" risk engine
  • telemetry on the ml_risk_score* indices
  • all internal APIs related to the legacy risk engine

@hop-dev hop-dev changed the title Delete legacy risk engine [Entity Analytics][9.0] Remove all legacy risk engine code Dec 2, 2024
@hop-dev hop-dev self-assigned this Dec 2, 2024
@hop-dev hop-dev added release_note:deprecation backport:skip This commit does not require backporting v9.0.0 Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Entity Analytics Security Entity Analytics Team labels Dec 2, 2024
@hop-dev
Copy link
Contributor Author

hop-dev commented Dec 2, 2024

/ci

@hop-dev
Copy link
Contributor Author

hop-dev commented Dec 2, 2024

/ci

@hop-dev
Copy link
Contributor Author

hop-dev commented Dec 3, 2024

/ci

@hop-dev
Copy link
Contributor Author

hop-dev commented Dec 3, 2024

/ci

@hop-dev hop-dev requested review from machadoum and removed request for CAWilson94 January 6, 2025 11:31
@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod bot requested a review from a team as a code owner January 6, 2025 11:38
machadoum
machadoum reviewed Jan 6, 2025
View reviewed changes
Copy link
Member

@machadoum machadoum left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this should be deleted:

kibana/x-pack/solutions/security/plugins/security_solution/public/entity_analytics/lens_attributes/risk_score_over_time_area.ts

Line 183 in da25d13

title: `ml_${stackByField}_risk_score_${extraOptions.spaceId}`,

I also found some references inside the cypress folder:

kibana/x-pack/test/security_solution_cypress/cypress/tasks/risk_scores/indices.ts

Line 11 in da25d13

`ml_${riskScoreEntity}_risk_score_${spaceId}`;

kibana/x-pack/test/security_solution_cypress/cypress/tasks/risk_scores/transforms.ts

Line 27 in da25d13

) => `ml_${riskScoreEntity}riskscore_pivot_transform_${spaceId}`;

kibana/x-pack/test/security_solution_cypress/cypress/tasks/risk_scores/stored_scripts.ts

Line 13 in da25d13

`ml_${riskScoreEntity}riskscore_init_script_${spaceId}`;

...

machadoum
machadoum approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I desk-tested, and everything looks good!

Thank you for deleting so much code! 🔥 🔥 🔥

PhilippeOberti
PhilippeOberti approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for the Threat Hunting Investigations team

angorayc
angorayc approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@angorayc angorayc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for removing the unused code!

@vitaliidm vitaliidm removed their request for review January 7, 2025 09:33
@hop-dev hop-dev requested a review from MadameSheema January 8, 2025 20:13
MadameSheema
MadameSheema reviewed Jan 9, 2025
View reviewed changes
...ty_solution_cypress/cypress/e2e/entity_analytics/dashboards/enable_risk_score_redirect.cy.ts Show resolved Hide resolved
...y_solution_cypress/cypress/e2e/entity_analytics/dashboards/entity_analytics/risk_score.cy.ts
cy.get(ENABLE_HOST_RISK_SCORE_BUTTON).should('be.visible');

cy.get(ENABLE_USER_RISK_SCORE_BUTTON).should('be.visible');
it('shows enable risk button', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we can invest time to try to unskip this spec file :)

x-pack/test/security_solution_cypress/cypress/e2e/entity_analytics/enrichments.cy.ts
});

after(() => {
cy.task('esArchiverUnload', { archiveName: 'risk_users' });
});

describe('Custom query rule', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, it would be great to try to unskip the test :)

x-pack/test/security_solution_cypress/cypress/e2e/entity_analytics/host_details/risk_tab.cy.ts Outdated Show resolved Hide resolved
x-pack/test/security_solution_cypress/cypress/e2e/entity_analytics/hosts/host_risk_tab.cy.ts Outdated Show resolved Hide resolved
...ck/test/security_solution_cypress/cypress/e2e/entity_analytics/hosts/hosts_risk_column.cy.ts Outdated Show resolved Hide resolved
nkhristinin
nkhristinin approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DE changes LGTM!

MadameSheema
MadameSheema approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing all the comments :)

@hop-dev
Copy link
Contributor Author

hop-dev commented Jan 10, 2025

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6541 6445 -96

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 22.2MB 21.1MB -1.1MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 88.2KB 87.3KB -882.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 575 573 -2

References to deprecated APIs

id before after diff
securitySolution 464 355 -109

Total ESLint disabled count

id before after diff
securitySolution 659 657 -2

History

cc @hop-dev

@hop-dev hop-dev merged commit 80baa2c into elastic:main Jan 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MadameSheema MadameSheema MadameSheema approved these changes

@PhilippeOberti PhilippeOberti PhilippeOberti approved these changes

@Ikuni17 Ikuni17 Ikuni17 approved these changes

@nkhristinin nkhristinin nkhristinin approved these changes

@angorayc angorayc angorayc approved these changes

@afharo afharo afharo approved these changes

@machadoum machadoum machadoum approved these changes

Assignees

@hop-dev hop-dev

Labels
backport:skip This commit does not require backporting release_note:deprecation Team:Entity Analytics Security Entity Analytics Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@hop-dev @elasticmachine @machadoum @afharo @angorayc @nkhristinin @Ikuni17 @PhilippeOberti @MadameSheema @kibanamachine